test: Use tmp_path pytest fixture over tmpdir #2384

matthewfeickert · 2023-11-20T17:51:04Z

Description

Resolves #2364

Use the tmp_path pytest fixtures as recommended in Issue #2634.

This PR originally tried to use the legacypath pytest plugin to disallow use of tmpdir but there are problems with pytest-mpl for making this happen.

c.f. https://docs.pytest.org/en/7.4.x/how-to/tmp_path.html#the-tmpdir-and-tmpdir-factory-fixtures

Checklist Before Requesting Reviewer

Tests are passing
"WIP" removed from the title of the pull request
Selected an Assignee for the PR to be responsible for the log summary

Before Merging

For the PR Assignees:

Summarize commit messages into a comprehensive review of the PR

* Use `tmp_path` over `tmpdir`.
* Use pathlib.Path functions now that paths and not files are being used.
   - Remove use of `.strpath`.
   - Update from `.join` to `.joinpath`.
   - For `json.loads` update from `.read` to `.read_text`.
   - For `json.load` update to use `.open`.
   - Update from `.write` to `.write_text` for patches.
   - Update from `.mkdir` to creating the path and then calling `.mkdir`.

tests/test_scripts.py

matthewfeickert · 2023-11-20T17:54:00Z

Once CI is passing should also add

pytest -p no:legacypath

to make sure that everything has gotten properly switched over.

codecov · 2023-11-20T23:25:49Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (f78442a) 98.28% compared to head (6e59ac7) 98.28%.

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #2384   +/-   ##
=======================================
  Coverage   98.28%   98.28%           
=======================================
  Files          69       69           
  Lines        4539     4539           
  Branches      803      803           
=======================================
  Hits         4461     4461           
  Misses         45       45           
  Partials       33       33

Flag	Coverage Δ
contrib	`97.86% <ø> (ø)`
doctest	`60.71% <ø> (ø)`
unittests-3.10	`96.29% <ø> (ø)`
unittests-3.11	`96.29% <ø> (ø)`
unittests-3.8	`96.32% <ø> (ø)`
unittests-3.9	`96.34% <ø> (ø)`

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

This reverts commit f853a6a.

matthewfeickert · 2023-11-21T03:51:05Z

Hm, seems that there's an issue with the pytest-mpl plugin given that using

[tool.pytest.ini_options]
...
addopts = [
    ...,
    "-p no:legacypath",
]

causes

$ pytest tests/contrib/test_viz.py --mpl --mpl-baseline-path tests/contrib/baseline/ -k 'test_plot_results_no_axis'

to error out with AttributeError: 'Function' object has no attribute 'fspath'

Output:

================================================================================= test session starts =================================================================================
platform linux -- Python 3.11.1, pytest-7.4.3, pluggy-1.3.0
benchmark: 4.0.0 (defaults: timer=time.perf_counter disable_gc=False min_rounds=5 min_time=0.000005 max_time=1.0 calibration_precision=10 warmup=False warmup_iterations=100000)
Matplotlib: 3.8.2
Freetype: 2.6.1
rootdir: /home/feickert/Code/GitHub/pyhf
configfile: pyproject.toml
plugins: anyio-3.6.2, benchmark-4.0.0, console-scripts-1.4.1, socket-0.6.0, mock-3.12.0, mpl-0.16.1, requests-mock-1.10.0
collected 9 items / 8 deselected / 1 selected                                                                                                                                         

tests/contrib/test_viz.py F                                                                                                                                                     [100%]

====================================================================================== FAILURES =======================================================================================
______________________________________________________________________________ test_plot_results_no_axis ______________________________________________________________________________

self = <pytest_mpl.plugin.ImageComparison object at 0x7f0f3af0d910>, item = <Function test_plot_results_no_axis>

    @pytest.hookimpl(hookwrapper=True)
    def pytest_runtest_call(self, item):  # noqa
    
        compare = get_compare(item)
    
        if compare is None:
            yield
            return
    
        import matplotlib.pyplot as plt
        try:
            from matplotlib.testing.decorators import remove_ticks_and_titles
        except ImportError:
            from matplotlib.testing.decorators import ImageComparisonTest as MplImageComparisonTest
            remove_ticks_and_titles = MplImageComparisonTest.remove_text
    
        style = compare.kwargs.get('style', 'classic')
        remove_text = compare.kwargs.get('remove_text', False)
        backend = compare.kwargs.get('backend', 'agg')
    
        with plt.style.context(style, after_reset=True), switch_backend(backend):
    
            # Run test and get figure object
            wrap_figure_interceptor(self, item)
            yield
            test_name = generate_test_name(item)
            if test_name not in self.return_value:
                # Test function did not complete successfully
                return
            fig = self.return_value[test_name]
    
            if remove_text:
                remove_ticks_and_titles(fig)
    
            result_dir = self.make_test_results_dir(item)
    
            summary = {
                'status': None,
                'image_status': None,
                'hash_status': None,
                'status_msg': None,
                'baseline_image': None,
                'diff_image': None,
                'rms': None,
                'tolerance': None,
                'result_image': None,
                'baseline_hash': None,
                'result_hash': None,
            }
    
            # What we do now depends on whether we are generating the
            # reference images or simply running the test.
            if self.generate_dir is not None:
                summary['status'] = 'skipped'
                summary['image_status'] = 'generated'
                summary['status_msg'] = 'Skipped test, since generating image.'
                generate_image = self.generate_baseline_image(item, fig)
                if self.results_always:  # Make baseline image available in HTML
                    result_image = (result_dir / "baseline.png").absolute()
                    shutil.copy(generate_image, result_image)
                    summary['baseline_image'] = \
                        result_image.relative_to(self.results_dir).as_posix()
    
            if self.generate_hash_library is not None:
                summary['hash_status'] = 'generated'
                image_hash = self.generate_image_hash(item, fig)
                self._generated_hash_library[test_name] = image_hash
                summary['baseline_hash'] = image_hash
    
            # Only test figures if not generating images
            if self.generate_dir is None:
                # Compare to hash library
                if self.hash_library or compare.kwargs.get('hash_library', None):
                    msg = self.compare_image_to_hash_library(item, fig, result_dir, summary=summary)
    
                # Compare against a baseline if specified
                else:
>                   msg = self.compare_image_to_baseline(item, fig, result_dir, summary=summary)

backend    = 'agg'
compare    = Mark(name='mpl_image_compare', args=(), kwargs={})
fig        = <Figure size 640x480 with 1 Axes>
item       = <Function test_plot_results_no_axis>
plt        = <module 'matplotlib.pyplot' from '/home/feickert/.pyenv/versions/pyhf-dev-cpu/lib/python3.11/site-packages/matplotlib/pyplot.py'>
remove_text = False
remove_ticks_and_titles = <function remove_ticks_and_titles at 0x7f0f39a00ea0>
result_dir = PosixPath('/tmp/tmpm5iex5fn/test_viz.test_plot_results_no_axis')
self       = <pytest_mpl.plugin.ImageComparison object at 0x7f0f3af0d910>
style      = 'classic'
summary    = {'baseline_hash': None, 'baseline_image': None, 'diff_image': None, 'hash_status': None, ...}
test_name  = 'test_viz.test_plot_results_no_axis'

../../../.pyenv/versions/pyhf-dev-cpu/lib/python3.11/site-packages/pytest_mpl/plugin.py:676: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
../../../.pyenv/versions/pyhf-dev-cpu/lib/python3.11/site-packages/pytest_mpl/plugin.py:457: in compare_image_to_baseline
    baseline_image_ref = self.obtain_baseline_image(item, result_dir)
        compare    = Mark(name='mpl_image_compare', args=(), kwargs={})
        compare_images = <function compare_images at 0x7f0f39a00c20>
        fig        = <Figure size 640x480 with 1 Axes>
        imread     = <function imread at 0x7f0f39c771a0>
        item       = <Function test_plot_results_no_axis>
        result_dir = PosixPath('/tmp/tmpm5iex5fn/test_viz.test_plot_results_no_axis')
        savefig_kwargs = {}
        self       = <pytest_mpl.plugin.ImageComparison object at 0x7f0f3af0d910>
        summary    = {'baseline_hash': None, 'baseline_image': None, 'diff_image': None, 'hash_status': None, ...}
        tolerance  = 2
../../../.pyenv/versions/pyhf-dev-cpu/lib/python3.11/site-packages/pytest_mpl/plugin.py:395: in obtain_baseline_image
    baseline_dir = self.get_baseline_directory(item)
        filename   = 'test_plot_results_no_axis.png'
        item       = <Function test_plot_results_no_axis>
        self       = <pytest_mpl.plugin.ImageComparison object at 0x7f0f3af0d910>
        target_dir = PosixPath('/tmp/tmpm5iex5fn/test_viz.test_plot_results_no_axis')
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _

self = <pytest_mpl.plugin.ImageComparison object at 0x7f0f3af0d910>, item = <Function test_plot_results_no_axis>

    def get_baseline_directory(self, item):
        """
        Return a full path to the baseline directory, either local or remote.
    
        Using the global and per-test configuration return the absolute
        baseline dir, if the baseline file is local else return base URL.
        """
        compare = get_compare(item)
        baseline_dir = compare.kwargs.get('baseline_dir', None)
        if baseline_dir is None:
            if self.baseline_dir is None:
                baseline_dir = Path(item.fspath).parent / 'baseline'
            else:
                if self.baseline_relative_dir:
                    # baseline dir is relative to the current test
                    baseline_dir = Path(item.fspath).parent / self.baseline_relative_dir
                else:
                    # baseline dir is relative to where pytest was run
                    baseline_dir = self.baseline_dir
    
        baseline_remote = (isinstance(baseline_dir, str) and  # noqa
                           baseline_dir.startswith(('http://', 'https://')))
        if not baseline_remote:
>           return Path(item.fspath).parent / baseline_dir
E           AttributeError: 'Function' object has no attribute 'fspath'

baseline_dir = '/home/feickert/Code/GitHub/pyhf/tests/contrib/baseline'
baseline_remote = False
compare    = Mark(name='mpl_image_compare', args=(), kwargs={})
item       = <Function test_plot_results_no_axis>
self       = <pytest_mpl.plugin.ImageComparison object at 0x7f0f3af0d910>

../../../.pyenv/versions/pyhf-dev-cpu/lib/python3.11/site-packages/pytest_mpl/plugin.py:363: AttributeError
-------------------------------------------------------------------------------- Captured stderr setup --------------------------------------------------------------------------------
INFO:root:copying /home/feickert/Code/GitHub/pyhf/tests/contrib/test_viz/tail_probs_hypotest_results.json -> /tmp/pytest-of-feickert/pytest-109/test_plot_results_no_axis0
INFO:root:copying /home/feickert/Code/GitHub/pyhf/tests/contrib/test_viz/hypotest_results.json -> /tmp/pytest-of-feickert/pytest-109/test_plot_results_no_axis0
--------------------------------------------------------------------------------- Captured log setup ----------------------------------------------------------------------------------
INFO     root:file_util.py:137 copying /home/feickert/Code/GitHub/pyhf/tests/contrib/test_viz/tail_probs_hypotest_results.json -> /tmp/pytest-of-feickert/pytest-109/test_plot_results_no_axis0
INFO     root:file_util.py:137 copying /home/feickert/Code/GitHub/pyhf/tests/contrib/test_viz/hypotest_results.json -> /tmp/pytest-of-feickert/pytest-109/test_plot_results_no_axis0
=============================================================================== short test summary info ===============================================================================
FAILED tests/contrib/test_viz.py::test_plot_results_no_axis - AttributeError: 'Function' object has no attribute 'fspath'

So we can't use -p no:legacypath until pytest-mpl fixes things on their side I think.

matthewfeickert · 2023-11-21T22:16:34Z

Might be able to put the -p no:legacypath back in soon if there is a pytest-mpl v0.17.0 release soon (c.f. matplotlib/pytest-mpl#207 (comment)).

matthewfeickert · 2023-11-21T23:14:36Z

I'm going to approve and merge this myself. As always, PRs approved by a single core dev can be reverted as needed by the rest of the dev team.

matthewfeickert added 9 commits November 19, 2023 00:46

test: Update tmpdir to tmp_path

c88a007

pathlib uses joinpath not join

68a5187

temp.strpath to temp

df33624

tempout.strpath to tempout

238da40

remove use of .strpath:s in f-strings

78e881b

Remove all use of .strpath

e6374dc

Use write_text and read_text

60d1752

fix this: The '-' isn't working apparently

836bb66

use exist_ok for mkdir

4d34dbd

matthewfeickert added tests pytest chore Other changes that don't modify src or test files labels Nov 20, 2023

matthewfeickert self-assigned this Nov 20, 2023

matthewfeickert commented Nov 20, 2023

View reviewed changes

tests/test_scripts.py Show resolved Hide resolved

Use patch.open() instead of .read_text()

6347dbd

matthewfeickert force-pushed the test/switch-to-tmp_path branch from 8309be1 to 6347dbd Compare November 20, 2023 23:15

Use nolegacypath plugin

f853a6a

matthewfeickert marked this pull request as ready for review November 20, 2023 23:28

matthewfeickert added 4 commits November 20, 2023 19:04

Remove .strpath

7f8802d

switch from join to joinpath

4bd2dd3

Remove use of Path directly

cbd3efb

Revert "Use nolegacypath plugin"

a6ffd65

This reverts commit f853a6a.

Restore noxfile

7d08e03

matthewfeickert requested review from kratsg and henryiii November 21, 2023 04:09

matthewfeickert mentioned this pull request Nov 21, 2023

Update to using pytest's tmp_path over legacy tmpdir matplotlib/pytest-mpl#209

Closed

use joinpath for notebooks

6e59ac7

matthewfeickert merged commit a1a31f1 into main Nov 21, 2023
24 checks passed

matthewfeickert deleted the test/switch-to-tmp_path branch November 21, 2023 23:34

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

test: Use tmp_path pytest fixture over tmpdir #2384

test: Use tmp_path pytest fixture over tmpdir #2384

matthewfeickert commented Nov 20, 2023 •

edited

Loading

matthewfeickert commented Nov 20, 2023

codecov bot commented Nov 20, 2023 •

edited

Loading

matthewfeickert commented Nov 21, 2023

matthewfeickert commented Nov 21, 2023

matthewfeickert commented Nov 21, 2023

test: Use tmp_path pytest fixture over tmpdir #2384

test: Use tmp_path pytest fixture over tmpdir #2384

Conversation

matthewfeickert commented Nov 20, 2023 • edited Loading

Description

Checklist Before Requesting Reviewer

Before Merging

matthewfeickert commented Nov 20, 2023

codecov bot commented Nov 20, 2023 • edited Loading

Codecov Report

matthewfeickert commented Nov 21, 2023

matthewfeickert commented Nov 21, 2023

matthewfeickert commented Nov 21, 2023

matthewfeickert commented Nov 20, 2023 •

edited

Loading

codecov bot commented Nov 20, 2023 •

edited

Loading